Address Copilot PR #1 feedback + document required OAuth scopes#2
Merged
Conversation
- Quote argument-hint as a YAML scalar in 8 SKILL.md files (was a bare
flow sequence [...], violating CONTRIBUTING.md frontmatter convention)
- intake.py: fail with a clear error on missing/invalid gws JSON instead
of an unhelpful ValueError/JSONDecodeError stack trace
- clean.py: tighten the Issues "bad email" rule from SEARCH("@") to a
REGEXMATCH so foo@ / foo@bar are flagged
- check_frontmatter.py: read via a context manager (no ResourceWarning)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Follow-up PR that applies the previously suggested inline fixes from PR #1, focusing on frontmatter/YAML correctness and small robustness improvements in the bundled Python scripts.
Changes:
- Quote
argument-hintin multipleSKILL.mdfrontmatter blocks so it remains a single YAML scalar string. - Make
aaif-triage-intake’sgwsJSON parsing more defensive with clearer failure messages. - Tighten the “bad email” Issues rule in
aaif-clean-dataand avoid aResourceWarningin the frontmatter checker.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| skills/aaif-triage-intake/scripts/intake.py | Safer detection/parsing of gws JSON output with clearer error messages on missing/invalid JSON. |
| skills/aaif-clean-data/scripts/clean.py | Strengthens the Google Sheets email validation formula using REGEXMATCH to require a domain + TLD. |
| scripts/check_frontmatter.py | Uses a context manager for file reads to avoid leaking file descriptors. |
| skills/aaif-announcement-post/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-attendee-reminder/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-carousel-copy/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-dayof-slides/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-luma-description/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-recap-post/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-speaker-bio/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
| skills/aaif-speaker-invite/SKILL.md | Quotes argument-hint to enforce valid YAML frontmatter. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…EADME Enabling the APIs is not enough — the ops skills write to Sheets/Drive and read Form responses, so the token needs read-write scopes. Read-only access passes the verify step but fails on the first write. Add Forms API + the forms.body / forms.responses.readonly scopes for intake ops. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rparundekar
changed the title
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #1 merged before its Copilot review comments were addressed. This follow-up applies all 11 of Copilot's inline findings, and additionally documents the OAuth scopes the ops skills require.
Changesets
Convention — quote
argument-hint(8 files)Copilot flagged
argument-hintwritten as a bare YAML flow sequence ([...]), which violates the repo convention (CONTRIBUTING.md) to fully-quote the value so it stays a single scalar string. Single-quoted in:aaif-announcement-post,aaif-attendee-reminder,aaif-carousel-copy,aaif-dayof-slides,aaif-luma-description,aaif-recap-post,aaif-speaker-bio,aaif-speaker-invite.Robustness (3 files)
skills/aaif-triage-intake/scripts/intake.py—fetch()previously didjson.loads(txt[txt.index("{"):]), crashing with an unhelpfulValueError/JSONDecodeErrorwhengwsreturns a non-JSON banner, empty output, or a different format. Now finds the JSON start safely and exits with a clear message on missing/invalid JSON.skills/aaif-clean-data/scripts/clean.py— the Issues "bad email" rule usedISNUMBER(SEARCH("@", …)), which passesfoo@andfoo@bar. Tightened to aREGEXMATCHrequiringlocal@domain.tld.scripts/check_frontmatter.py— read the file via a context manager instead ofopen(...).read()(avoidsResourceWarning).Docs — required OAuth scopes (README)
Enabling the Google APIs alone is insufficient: the ops skills write to Sheets/Drive and read Form responses. Documented the read-write scopes (
spreadsheets,drive) plus Forms (forms.body,forms.responses.readonly) and added the Forms API to the enable list, with a note that read-only access passes the verify step but fails on the first write.Test Plan
py_compileclean on all 3 scriptscheck_frontmatter.pyparses all 12 SKILL.md blocks (quoted hints load as strings)pre-commit runpasses on all changed files (ruff, codespell, gitleaks, frontmatter)🤖 Generated with Claude Code